-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump github.com/hashicorp/terraform-plugin-framework from 0.16.0 to 0.17.0 #338
Bump github.com/hashicorp/terraform-plugin-framework from 0.16.0 to 0.17.0 #338
Conversation
Bumps [github.com/hashicorp/terraform-plugin-framework](https://github.com/hashicorp/terraform-plugin-framework) from 0.16.0 to 0.17.0. - [Release notes](https://github.com/hashicorp/terraform-plugin-framework/releases) - [Changelog](https://github.com/hashicorp/terraform-plugin-framework/blob/main/CHANGELOG.md) - [Commits](hashicorp/terraform-plugin-framework@v0.16.0...v0.17.0) --- updated-dependencies: - dependency-name: github.com/hashicorp/terraform-plugin-framework dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
… values are null in the bool, int64, map and string plan modifiers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this is looking really good, just some potential considerations. 👍
}, | ||
Optional: true, | ||
PlanModifiers: []tfsdk.AttributePlanModifier{ | ||
planmodifiers.RequiresReplaceIfValuesNotNull(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the old planmodifiers
package? 😄
// RequiresReplace returns an attribute plan modifier that is identical to resource.RequiresReplace() with | ||
// the exception that there is no check for `configRaw.IsNull && attrSchema.Computed` as a replacement | ||
// needs to be triggered when the attribute has been removed from the config. | ||
func RequiresReplace() planmodifier.String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this now can be removed with the resource/schema/stringplanmodifier.RequiresReplace()
variant since the underlying implementation no longer checks for configuration (that's done with the RequiresReplaceIfConfigured()
variant). 👍
} | ||
|
||
func (d *defaultValueAttributePlanModifier) Description(ctx context.Context) string { | ||
return "If the config does not contain a value, a default will be set using val." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we are adjusting these, it'd be great to fix them up so they would output the default value correctly, e.g.
return fmt.Sprintf("If not configured, defaults to %t", d.val.ValueBool())
Rinse and repeat for the others as appropriate.
We can also discuss whether we think we are ready to include something like this in the framework itself. 😄
// TODO: passwordSchemaV2 needs to be updated to use schema.Schema once resource.StateUpgrader has been | ||
// updated to use schema.Schema for PriorSchema. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aye, my bad for missing this -- cross reference: hashicorp/terraform-plugin-framework#572
We can certainly handle this TODO after framework v1.0.0 is out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me 👍
path.MatchRoot("min_special"), | ||
), | ||
}, | ||
// TODO: Reinstate once passwordSchemaV2 has been updated to use schema.Schema. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the resource state upgrade prior schemas aren't "live" schemas that would participate in ValidateResourceConfig
or PlanResourceChange
RPCs, we could also just drop PlanModifiers
and Validators
from them rather than worrying about converting this code. 👍
internal/provider/resource_pet.go
Outdated
int64planmodifiers.DefaultValue( | ||
types.Int64Value(2), | ||
), | ||
int64planmodifiers.RequiresReplace(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be replaced with the framework resource/schema/int64planmodifier.RequiresReplace()
?
internal/provider/resource_string.go
Outdated
boolplanmodifiers.DefaultValue( | ||
types.BoolValue(true), | ||
), | ||
boolplanmodifiers.RequiresReplace(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here and below regarding the usage of the framework-based plan modifiers
boolplanmodifiers.DefaultValue( | ||
types.BoolValue(true), | ||
), | ||
boolplanmodifiers.RequiresReplace(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 🚀
// RequiresReplace returns an attribute plan modifier that is identical to resource.RequiresReplace() with | ||
// the exception that there is no check for `configRaw.IsNull && attrSchema.Computed` as a replacement | ||
// needs to be triggered when the attribute has been removed from the config. | ||
func RequiresReplace() planmodifier.Bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready for the 🪓 now 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Bumps github.com/hashicorp/terraform-plugin-framework from 0.16.0 to 0.17.0.
Release notes
Sourced from github.com/hashicorp/terraform-plugin-framework's releases.
Changelog
Sourced from github.com/hashicorp/terraform-plugin-framework's changelog.
Commits
54ed5dc
Update CHANGELOG for 0.17.0d51781c
website: Updates for tfsdk Attribute, Block, and Schema deprecations (#564)4db7ec6
resource/schema: New packages which contain type-specific schema plan modifie...8cde922
tfsdk: Deprecate Attribute, Block, and Schema types (#563)9353b7c
provider/metaschema: Initial package (#562)abe43b2
Fix nesting mode for map, set and single nested attribute within data source,...55244fe
provider/schema: Fix Go documentation for Schema type (#559)30b78ab
resource/schema: Initial package (#558)28f4804
resource/schema/planmodifier: New type-specific plan modifiers package (#557)1dfcd30
test: block attributes with MarkComputedNilsAsUnknown (#555)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)